Fixed NPE for unknown request in FilterChainProxy #18166
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
Closes spring-projects#18157 Signed-off-by: Soumik Sarker <ronodhirsoumik@gmail.com>
ronodhirSoumik
force-pushed
the
bugfix/fixed-npe-in-FilterChainProxy
branch
from
539fa3e to
c8bd4d4
Compare
| String requestMethod = request.getMethod(); | ||
| return requestMethod != null && this.method.name().equals(requestMethod); |
There was a problem hiding this comment.
We don’t need to check for null because this.method.name() will return a String type and String#equals(null) will return false, so the test passes without any changes.
There was a problem hiding this comment.
but this fixes the NPE case - ig that is our expectation of getting false for invalid
There was a problem hiding this comment.
this.method.name().equals(request.getMethod()) will return false if request.getMethod() == null and won’t throw an NPE unless request is null or method is null. The test passes without any changes.
|
Thank you for your submission @ronodhirSoumik As pointed out, the test will pass without changes to the code. Additionally, the issue you are linking to is caused by ConcurrentHashMap not supporting a null key (which is of type ServletContext) and is unrelated to the method being null. I think that the test would still provide value by ensuring that we do not get a NullPointerException later. If you would modify the PR to just have the test and update the PR subject, I can merge it. |
rwinch
added
status: waiting-for-feedback
4 participants
Fixes #18157